perf(clients): fix composer draft persistence boundaries - #9049
perf(clients): fix composer draft persistence boundaries#9049StiensWout wants to merge 4 commits into
Conversation
Web: the zustand persist pipeline ran the full draft walk plus JSON.stringify on every store write, so each keystroke serialized every persisted draft, including base64 image attachments; only the final localStorage write was debounced. partialize now captures the live state cheaply and the walk + stringify run once per idle flush inside the debounced storage. 200 keystrokes next to two 1 MB images: 200 serializations / 534.6 MB / 1693.5 ms of main-thread time down to 1 / 2.7 MB / 6.8 ms. Mobile: picked and pasted images persisted full base64 data URLs into drafts.json, thread-outbox/*.json, and incoming-shares/*.json (an 8 MB image cost 21.3 MB of JSON per copy, rewritten on every 200 ms draft debounce). Images are now file-backed like file attachments already were: bytes are copied once into the app-owned attachment directory and drafts persist metadata plus fileUri (322 bytes). Uploads stream from the owned file; base64 is materialized lazily only for the old-server inline fallback. Legacy dataUrl drafts still hydrate, preview, upload, and send. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| name: input.name, | ||
| mimeType: input.mimeType, | ||
| sizeBytes, | ||
| fileUri, |
There was a problem hiding this comment.
🟡 Medium lib/composerImages.ts:290
Review-comment images leave their copied files in t3-composer-attachments permanently when removed or when the sheet is dismissed, steadily consuming device storage. createComposerImageAttachment returns an app-owned fileUri, but the review-comment composer only drops the attachment from state instead of calling removePersistedComposerAttachmentFile; invoke that cleanup path for both removal and dismissal.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/lib/composerImages.ts around line 290:
Review-comment images leave their copied files in `t3-composer-attachments` permanently when removed or when the sheet is dismissed, steadily consuming device storage. `createComposerImageAttachment` returns an app-owned `fileUri`, but the review-comment composer only drops the attachment from state instead of calling `removePersistedComposerAttachmentFile`; invoke that cleanup path for both removal and dismissal.
There was a problem hiding this comment.
[claude-fable-5] RESPONDING ON BEHALF OF WOUT:
Confirmed and fixed in afe1617. The sheet's local attachments now go through the reference-checked cleanup: removing an attachment releases its copied file immediately, and any attachments still held when the sheet unmounts (dismissal by cancel, back, or gesture) are released there. Submitted attachments are appended to the thread draft before the sheet closes, so the reference check keeps their files.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
The review-comment sheet holds picked images in local state until submit copies them into the thread draft. With file-backed images, removing an attachment or dismissing the sheet dropped the state while the copied file stayed in the attachment directory. Removal and unmount now route through the reference-checked cleanup, so submitted files stay owned by the draft and abandoned ones are deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This is an XL cross-platform persistence and attachment-lifecycle migration, not a small performance-only change: it adds app-owned image files, asynchronous upload fallbacks, outbox v4 handling, HEIC conversion, and deferred web serialization. The unresolved medium finding concerns cleanup of abandoned review-comment files, while downgrade behavior also changes, so the storage lifecycle and compatibility boundaries need human verification. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit afe1617. Configure here.
Dropping the picker's base64 export also dropped the only source of provider-supported bytes for HEIC-family originals: the picker's file copy stays HEIC on both its fast and slow paths, so those photos were rejected as unsupported. The picker exports JPEG base64 again, used only as the fallback for unsupported originals and landed once in the owned attachment directory; supported formats still copy their original file and nothing base64 is persisted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
size:XL
iOS always transcodes the base64 export to JPEG, but Android's quality-1 export is the raw original, so an Android HEIC pick would have shipped HEIC bytes labeled image/jpeg. The fallback now requires the JPEG magic number in the export and otherwise rejects the photo as unsupported, matching the pre-change Android behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Defer the draft walk and JSON serialization until the storage write flushes. Preserve hydration, migrations, attachment verification, and final flushes. Continues the web portion of [#9049](#9049). The mobile storage migration remains separate. Created with GPT-6 Astra (preview) in Codex. Co-authored-by: Wout Stiens <71498452+StiensWout@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Typing next to a heavy composer draft was paying for the whole draft store on every keystroke, and mobile image drafts were storing megabytes of base64 in JSON. This is item 03 of the performance opportunity audit.
Web: the persist pipeline ran the full draft walk plus
JSON.stringifyon every store write, so each keystroke serialized every persisted draft, including base64 image attachments; only the finallocalStorage.setItemwas debounced.partializenow captures the live state reference cheaply, and the real walk + stringify run once per idle flush inside the debounced storage (createDeferredStorage). Flush-on-unload, flush-before-attachment-verify, migration writeback, and hydration behavior are unchanged.Mobile: picked and pasted images persisted full base64 data URLs into
drafts.json,thread-outbox/*.json, andincoming-shares/*.json, re-serialized on every 200 ms draft debounce and re-parsed at cold start. Images are now file-backed the way file attachments already were: bytes are copied once into the app-owned attachment directory and drafts persist metadata plusfileUri. Uploads stream from the owned file (no temp base64 staging), and base64 is materialized lazily only for the old-server inline fallback. LegacydataUrldrafts still hydrate, preview, upload, and send; the outbox schema version moves 3 → 4 following the existing pattern.Measurements
Web numbers are from a store seeded with 20 drafts, one holding two 1 MB persisted images, driving 200
setPromptcalls through the real persist pipeline. Mobile numbers encode one attachment through the actual draft schema (the legacy shape stored the data URL twice:dataUrl+previewUri).Draft persistence still works (type → reload → restored)
Recorded against a live dev environment running this branch: the draft is typed, flushed after idle, and restored into the composer and sidebar after a full page reload.
Verification
vp test runacross all 12 touched suites: 333 tests pass (web draft store 105, mobile attachment/draft/outbox/share suites 228).tsgo --noEmitfor web,tsc --noEmitfor mobile) andvp linton changed files: clean.Known limitation: downgrading the mobile app to a pre-change build will not decode fileUri-only drafts or v4 outbox entries, the same class of impact as the earlier file-attachment rollout.
Change made by Claude Fable 5 running in Claude Code.
Note
Medium Risk
Broad changes to mobile attachment lifecycle, outbox schema v4, and async image upload wiring; legacy paths are preserved but downgrading the mobile app will not read file-only drafts.
Overview
Web: Composer draft persistence no longer runs the full draft walk and
JSON.stringifyon every keystroke.partializeonly captures a live state reference;createDeferredStorageruns normalization and serialization once per debounced flush (flush-on-unload unchanged).Mobile: New image attachments (picker, clipboard, paste, incoming share) are stored as app-owned files with
fileUrimetadata instead of megabyte-scaledataUrlin drafts, thread outbox (schema v3 → v4), and share drafts. Uploads stream from the owned file; base64 is read lazily only for servers without image uploads. Legacy inlinedataUrldrafts still decode and send. Image previews rebase paths when the iOS document container moves; review composer cleans up unreferenced files on dismiss/remove.Send path:
buildProjectThreadStartTurnInputnow always uses prepareduploadedAttachments(raw draft attachments removed from project start/outbox drain).Reviewed by Cursor Bugbot for commit 0c1f781. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Switch composer image attachments to app-owned files
dataUrlimage storage with app-owned file paths (fileUri) across composer drafts, outbox, and incoming shares.FileBackedComposerAttachmenttype andisFileBackedComposerAttachmentguard to narrow attachments with a definedfileUri.toUploadChatImageAttachmentsis now async and lazily reads base64 from disk only when the server lacks image uploads.uploadFileBytesuploads directly fromfileUriand only stages temporary files for legacy inline images.createDeferredStoragewith deferred serialization;partializeComposerDraftStoreStateis now exported.dataUrlorfileUri; old drafts withoutfileUristill work via legacy inline path. Drafts/outbox now persist image paths instead of bytes.Macroscope summarized 0c1f781.